Better types from jsdoc type references #16316
Conversation
src/compiler/checker.ts
Outdated
| @@ -6814,37 +6814,58 @@ namespace ts { | |||
| return undefined; | |||
| } | |||
|
|
|||
| function resolveTypeReferenceName(typeReferenceName: EntityNameExpression | EntityName) { | |||
| function resolveTypeReferenceName(node: TypeReferenceType, typeReferenceName: EntityNameExpression | EntityName) { | |||
There was a problem hiding this comment.
This function is only called in one location. At that location, we have special handling for JSDocTypeReference already. So maybe we should just avoid calling resolveTypeReferenceName from that code path altogether instead of adding special-casing for jsdoc here. Same for getTypeReferenceName.
There was a problem hiding this comment.
Also, getPrimitiveTypeFromJSDocTypeReference may return undefined if the jsdoc type was not a primitive type name, thus even JSDocTypeReference nodes will call this function.
src/compiler/checker.ts
Outdated
| } | ||
|
|
||
| function getTypeReferenceType(node: TypeReferenceType, symbol: Symbol) { | ||
| const typeArguments = typeArgumentsFromTypeReferenceNode(node); // Do unconditionally so we mark type arguments as referenced. | ||
| let secondPass = true; |
There was a problem hiding this comment.
How can we be on the second pass already?
There was a problem hiding this comment.
...which is why the test is failing. Last minute typo :/
src/compiler/checker.ts
Outdated
| } | ||
|
|
||
| function getTypeReferenceType(node: TypeReferenceType, symbol: Symbol) { | ||
| const typeArguments = typeArgumentsFromTypeReferenceNode(node); // Do unconditionally so we mark type arguments as referenced. | ||
| let secondPass = true; | ||
| let fallbackType: Type = unknownType; | ||
| while (true) { |
There was a problem hiding this comment.
Since this loop has a maximum of 2 iterations, couldn't you just define a function and call it twice?
There was a problem hiding this comment.
I could. I could also call this function recursively. This approach avoids call stack allocation since I reuse the current stack frame.
src/compiler/checker.ts
Outdated
| return getTypeFromNonGenericTypeReference(node, symbol); | ||
| // Try to use the symbol of the type (if present) to get a better type on the | ||
| // second pass. | ||
| fallbackType = getTypeOfSymbol(symbol); |
There was a problem hiding this comment.
Is this intended to allow:
const x = new Foo();
// Equivalent to @return {Foo}
/** @return {x} */
function f() { return x; }There was a problem hiding this comment.
This needs to continue to be reported as an error, but we will fall back to the type of the value as per the existing comment.
Previously, if the symbol for the type reference could not be resolved to a type and the symbol pointed to a value, we would just get the type of the symbol. However, this practice results in the wrong type for a JavaScript constructor function.
With this change, we now try to follow the value symbol across CommonJS
requirecalls to get the correct symbol. If the resulting symbol is a JavaScript constructor function, we return the inferred class type for the symbol. Otherwise, we fall back to the previous behavior.Fixes #14056